Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/add timers core250 #2182

Merged
merged 2 commits into from
Dec 19, 2018
Merged

Conversation

TD-er
Copy link
Member

@TD-er TD-er commented Dec 19, 2018

As requested here: esp8266/Arduino#5513 (comment)

@TD-er TD-er merged commit 708adf2 into letscontrolit:mega Dec 19, 2018
@TD-er TD-er deleted the feature/AddTimers_core250 branch December 19, 2018 15:05
@uzi18
Copy link
Contributor

uzi18 commented Dec 23, 2018

@TD-er got build problem:

/home/users/linaja/ESPEasy-mega_test/src/__CPlugin.ino: In function 'void CPluginInit()':
/home/users/linaja/ESPEasy-mega_test/src/__CPlugin.ino:19:100: error: invalid conversion from 'boolean (*)(byte, EventStruct*, String&) {aka unsigned char (*)(unsigned char, EventStruct*, String&)}' to 'bool (*)(byte, EventStruct*, String&) {aka bool (*)(unsigned char, EventStruct*, String&)}' [-fpermissive]
#define ADDCPLUGIN(NNN) if (x < CPLUGIN_MAX) { CPlugin_id[x] = CPLUGIN_ID_##NNN; CPlugin_ptr[x++] = &CPlugin_##NNN; } else addLog(LOG_LEVEL_ERROR, FPSTR(ADDCPLUGIN_ERROR));
^
/home/users/linaja/ESPEasy-mega_test/src/__CPlugin.ino:108:3: note: in expansion of macro 'ADDCPLUGIN'
ADDCPLUGIN(019)
^

@TD-er
Copy link
Member Author

TD-er commented Dec 23, 2018

Did you rebase it all?

I did change the boolean return value to bool for the CPlugin.
I have to go in a minute, so maybe you can see if I forgot to commit something?

@uzi18
Copy link
Contributor

uzi18 commented Dec 23, 2018

@TD-er ok sorry it was my plugin not changed to bool ;)

@uzi18
Copy link
Contributor

uzi18 commented Dec 23, 2018

@TD-er be sure more plugins not official (like my Nettemp support) will report now this issue, error is not straightforward to find because of macro used here ;)

@TD-er
Copy link
Member Author

TD-er commented Dec 23, 2018

That's why I didn't change it for the plug-in macro yet

@uzi18
Copy link
Contributor

uzi18 commented Dec 23, 2018

personally I would change it, but after stable release, it is not a big deal and consequences are bigger as it is hard to find compilation time issue

@TD-er
Copy link
Member Author

TD-er commented Dec 23, 2018

Yep, I agree.
Since there aren't many 'controllers' in the wild, I thought it wouldn't be such an issue, but for plugins it's much more of an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants